Skip to content

Comments

fix(stream): allow DeferredReadableStream to be reused after detachSource#1018

Closed
mshivam019 wants to merge 1 commit intolivekit:mainfrom
mshivam019:fix/deferred-stream-reconnection
Closed

fix(stream): allow DeferredReadableStream to be reused after detachSource#1018
mshivam019 wants to merge 1 commit intolivekit:mainfrom
mshivam019:fix/deferred-stream-reconnection

Conversation

@mshivam019
Copy link
Contributor

@mshivam019 mshivam019 commented Feb 3, 2026

Description

Fix DeferredReadableStream to allow setSource() to be called again after detachSource(), enabling stream reuse during participant reconnection scenarios.

When a participant's connection is interrupted and restored (e.g., browser reload, network interruption, or user canceling the "Leave site?" confirmation dialog), the SDK's ParticipantAudioInputStream attempts to reattach the audio track via onTrackSubscribed. This triggers:

  1. closeStream() → calls deferredStream.detachSource()
  2. Then deferredStream.setSource(newAudioStream) to attach the new track

However, setSource() throws "Stream source already set" because:

  1. detachSource() releases the reader lock but doesn't reset sourceReader to undefined
  2. isSourceSet (which checks !!this.sourceReader) still returns true
  3. Even if that were fixed, the writer may already be closed from the previous session

This makes it impossible to reuse the agent session after any participant reconnection event.

Changes Made

  • Reset sourceReader = undefined in detachSource() so isSourceSet returns false
  • Added writerClosed state tracking to detect when transform needs recreation
  • Recreate the internal transform stream in setSource() if writer was previously closed
  • Added check for !this.sourceReader before read in pump() to handle race condition where detachSource() is called while pump is running

Pre-Review Checklist

  • Build passes: All builds (lint, typecheck, tests) pass locally
  • AI-generated code reviewed: Removed unnecessary comments and ensured code quality
  • Changes explained: All changes are properly documented and justified above
  • Scope appropriate: All changes relate to the PR title
  • Video demo: N/A - This is an internal stream management fix, not user-facing UI. The fix enables reconnection scenarios that previously crashed.

Testing

  • Automated tests added/updated (if applicable) - All 25 existing tests in deferred_stream.test.ts pass
  • All tests pass - pnpm build:agents and pnpm test pass
  • Make sure both restaurant_agent.ts and realtime_agent.ts work properly - N/A for this internal stream fix

Manual Testing Performed:

  • Verified fix in production with browser tab close/cancel reconnection scenario
  • Verified fix with network interruption simulation
  • Confirmed agent session remains functional after participant reconnects

Additional Notes

This bug affects any scenario where a participant disconnects and reconnects to the same room session. Without this fix, the agent becomes unresponsive after reconnection because audio input cannot be reattached.


Note to reviewers: Please ensure the pre-review checklist is completed before starting your review.

…urce

When a user disconnects and reconnects (e.g., cancels browser 'Leave site?' dialog),
the agent's audio input stream would throw 'Stream source already set' error because:

1. detachSource() releases the reader lock but didn't reset sourceReader
2. isSourceSet still returns true, so setSource() throws
3. The writer may be closed from the previous session

This fix:
- Resets sourceReader = undefined in detachSource() so isSourceSet returns false
- Tracks writerClosed state and recreates transform in setSource() if needed
- Handles race condition where pump() may still run when detachSource() is called
@CLAassistant
Copy link

CLAassistant commented Feb 3, 2026

CLA assistant check
All committers have signed the CLA.

@changeset-bot
Copy link

changeset-bot bot commented Feb 3, 2026

⚠️ No Changeset found

Latest commit: d01e684

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 3, 2026

📝 Walkthrough

Walkthrough

The deferred stream implementation now properly handles reattachment of sources by introducing a writerClosed flag to track writer closure state. When reattaching, the IdentityTransform and writer are reinitialized if previously closed, while detached state handling and error treatment are improved for safer source attachment lifecycle management.

Changes

Cohort / File(s) Summary
Deferred Stream Reattachment Logic
agents/src/stream/deferred_stream.ts
Added writerClosed flag to track writer closure, reinitialize transform and writer on setSource if previously closed, handle detached state in pump, treat stream cleanup errors as detachment, and clarify detachSource behavior for safe reattachment.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

  • rektdeckard

Poem

🐰 A stream once broken, now springs to life,
When sources detach, then reattach with might!
The writer reborn from its shuttered state,
Hopping forward through transform's gate. 🌊

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely summarizes the main change: allowing DeferredReadableStream to be reused after detachSource, which is the primary objective of the PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description follows the template structure with clear Description, Changes Made, Pre-Review Checklist, Testing, and Additional Notes sections.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@toubatbrian
Copy link
Contributor

Hey @mshivam019, the DeferredReadableStream is intentionally designed to prevent calling setSource after detachSource. Here's why:

Consider an async task reading from this stream:

const deferred = new DeferredReadableStream<string>();
const reader = deferred.stream.getReader();

while (true) {
  const { value, done } = await reader.read();
  if (done) break;
  // process value...
}

Once detachSource is called, done becomes true and the read loop exits. This follows the standard ReadableStream contract—when done is true, the stream is fully closed and consumers should expect no further values. Re-attaching a source would violate this expectation.

Regarding your scenarios:

  • Browser reload: The previous room connection should be disconnected with no reconnection expected.
  • Network interruption: This triggers reconnecting events, but the stream itself shouldn't need re-attachment.

Could you clarify: have you observed this deferred readable stream blocking the agent's progress during reconnection? If so, could you share more details about the specific scenario? That would help us better understand if there's an edge case we need to address.

@toubatbrian toubatbrian self-requested a review February 4, 2026 23:40
@mshivam019
Copy link
Contributor Author

@toubatbrian Thanks for the detailed explanation of the design rationale!

Let me clarify my specific use case:

Scenario: Interview Session with Reconnection Grace Period

I'm building a voice AI interview application where users may accidentally:

  • Reload the page or navigate away briefly and return
  • Experience browser/device crashes

For this, I want a 3-minute grace window where the user can rejoin the same room and continue the conversation with full context preserved.

How I've configured this:

Room creation (backend):

await roomService.createRoom({
  name: roomName,
  metadata: roomMetadata,
  emptyTimeout: 300,      // 5 minutes before any participant joins
  departureTimeout: 180,  // 3 minutes grace period after last participant leaves
  maxParticipants: 10,
});

Agent session (keeps session alive when user disconnects):

await session.start({
  agent: assistant,
  room: ctx.room,
  inputOptions: {
    noiseCancellation: BackgroundVoiceCancellation(),
    closeOnDisconnect: false,  // Keep agent session alive for reconnection
  },
});

The Problem

When the user reconnects (same room, same identity):

  1. LiveKit fires TrackSubscribed for their new microphone track
  2. ParticipantAudioInputStream.onTrackSubscribed() calls closeStream()detachSource()
  3. Then immediately calls setSource(newAudioStream)
  4. Without this patch: Throws "Stream source already set"
  5. Result: Agent cannot hear user's audio (one-way audio - user hears agent, agent doesn't hear user)

Question

If making DeferredReadableStream reusable isn't the preferred approach, what would you recommend for this "reconnection with context preservation" scenario?

Should ParticipantAudioInputStream perhaps create a new DeferredReadableStream instance on reconnection rather than reusing the existing one? Or is there another pattern you'd suggest?

Happy to adjust the implementation based on your guidance!

@toubatbrian
Copy link
Contributor

Hey @mshivam019, I made a PR to support a new stream primitive, which could hopefully support the usecase you want: #1036.

Feel free to test on that branch and let me know if you encounter any issues!

@mshivam019
Copy link
Contributor Author

mshivam019 commented Feb 10, 2026

Hey @toubatbrian, thanks for the pointer! In the meantime, I migrated my architecture to create a fresh room on reconnection instead — the backend preloads the conversation context into the new agent session, so the user picks up right where they left off without needing stream reuse. That eliminated the need for this PR on my end.

That said, keeping the original room open would still be the better approach — with the new room architecture, there's a window where recent conversation data that's still in the agent's context but hasn't been persisted by the backend yet can get lost during the handoff. The stream reuse approach avoids that entirely since the session stays alive.

I checked out #1036 and the deferred stream approach looks solid. I'll give it a try soon and report back if I run into anything.

@lukasIO lukasIO mentioned this pull request Feb 16, 2026
@toubatbrian
Copy link
Contributor

I'm closing this PR since we've merged #1036

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants